feat(session-ingest): push notifications for remote session attention signals#4445
Conversation
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThe only changes since the last review mark five tests as Files Reviewed (2 files)
Previous Review Summaries (9 snapshots, latest commit 667e2af)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 667e2af)Status: No Issues Found | Recommendation: Merge Executive SummaryThe only change since the last review is a single blank-line removal in a test file; no functional or logic changes were introduced. Files Reviewed (1 file)
Previous review (commit 3f1b5c5)Status: No Issues Found | Recommendation: Merge Executive SummaryThis commit adds a kill switch ( Files Reviewed (2 files)
Note: With pushes globally disabled by this flag, the previously reported findings on Previous review (commit 408ce7d)Status: No Issues Found | Recommendation: Merge Executive SummaryThis commit only removes noisy logging and pushes item-offload filtering into the SQL query itself; no new issues were introduced in the incremental diff. Files Reviewed (4 files)
Note: Two previously reported findings remain open on unchanged code and are unaffected by this commit: Previous review (commit 7a7f89e)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (6 files)
Fix these issues in Kilo Cloud Previous review (commit 640c916)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (6 files)
Fix these issues in Kilo Cloud Previous review (commit 6e10e17)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (31 files)
Fix these issues in Kilo Cloud Previous review (commit 2362a7a)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe latest changes replace web-view push suppression with a presence-based check and add local-CLI-activity suppression, but the previously flagged eligibility gap (local terminal CLI sessions still qualify for pushes, not just VS Code/Agent Manager sessions) remains unresolved since Overview
Issue Details (click to expand)WARNING
Files Reviewed (16 files, incremental)
Fix these issues in Kilo Cloud Previous review (commit e9f3fd2)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe incremental changes replace the platform-based push eligibility check with an active-CLI-heartbeat check that no longer restricts remote-session attention pushes to VS Code/Agent Manager sessions, broadening pushes to local terminal CLI sessions as well. Overview
Issue Details (click to expand)WARNING
Files Reviewed (8 files, incremental)
Fix these issues in Kilo Cloud Previous review (commit 552691c)Status: No Issues Found | Recommendation: Merge Executive SummaryReviewed the new remote-session attention-push pipeline (SessionIngestDO signal detection, queue-consumer dispatch/partial-flush-on-failure, remote-session-notifications eligibility/body-building, and the UserConnectionDO subscriber check) with high confidence and found no security, correctness, or breaking-change issues in the changed code. Files Reviewed (14 files)
Generated files ( Reviewed by claude-sonnet-5 · Input: 20 · Output: 4.5K · Cached: 407.8K Review guidance: REVIEW.md from base branch |
552691c to
e9f3fd2
Compare
2362a7a to
db65f76
Compare
… signals Detect completed assistant turns and needs-input status transitions during ingest and dispatch a mobile push to the session owner for remote (vscode, agent-manager) root sessions. Pushes are suppressed when the user already has the session open in the web app (UserConnectionDO.hasSessionSubscribers). - SessionIngestDO emits AttentionSignal(s) on idle/question/permission transitions, pairing completed turns with the just-finished assistant message excerpt. First-ever status writes are skipped to avoid pushing about an old turn on full-history backfill. - queue-consumer collects signals across chunks and dispatches them via a NOTIFICATIONS service binding; partial flush on failure preserves signals from already-committed chunks. - Adds @kilocode/notifications dependency and wrangler NOTIFICATIONS binding.
db65f76 to
ca81a23
Compare
…lify dispatch paths - notifications: inline dispatchPushCore into dispatchPush, drop verbose console.log outcome logging in cloud-agent-session-push - session-ingest: push item_data_r2_key filter into SQL queries instead of filtering in JS, drop verbose logging in attention signal dispatch
…I presence reporting ships
…CLI presence reporting ships
Summary
Session-ingest now sends a mobile push when a remote root session needs attention: the assistant finishes a turn (
status -> idle) or the session is waiting for user input (status -> question/permission). This gives VS Code and Agent Manager users a signal to return to a remotely controllable session.Key behavior:
SessionIngestDO.ingest()emits attention signals alongside metadata changes. Completed signals include an excerpt from the latest completed assistant message; needs-input signals use a fixed prompt.queue-consumercollects signals across ingest chunks, persists metadata first, and dispatches pushes best-effort through the notifications service. Signals from already-committed chunks are also dispatched after a later chunk fails, because retries will not re-emit those transitions.completedstatus are reused for compatibility with deployed notification schemas.Verification
pnpm --filter notifications test -- src/lib/notifications-service-cloud-agent.test.tspnpm --filter notifications typecheckpnpm --filter notifications lintpnpm --filter cloudflare-session-ingest typecheckpnpm --filter cloudflare-session-ingest lintpnpm --filter @kilocode/event-service test -- src/__tests__/presence.test.tspnpm --filter @kilocode/event-service typecheckpnpm --filter web typecheckgit diff --checkVisual Changes
N/A
Reviewer Notes